Skip to content

feat(losses/metrics): implement ignore_index support across dice and …#8757

Open
Rusheel86 wants to merge 9 commits intoProject-MONAI:devfrom
Rusheel86:feat-ignore-index-support
Open

feat(losses/metrics): implement ignore_index support across dice and …#8757
Rusheel86 wants to merge 9 commits intoProject-MONAI:devfrom
Rusheel86:feat-ignore-index-support

Conversation

@Rusheel86
Copy link

@Rusheel86 Rusheel86 commented Feb 25, 2026

Fixes #8734

Description

This PR introduces comprehensive native support for ignore_index in core MONAI losses and metrics, as requested in issue #8734. The ignore_index parameter allows specific label values (e.g., padding, unlabeled regions, or boundary artifacts) to be excluded from loss and metric calculations which is a critical feature for medical imaging workflows.

Implementation Summary

Losses with ignore_index support:

  • DiceLoss - masks ignored voxels before dice computation
  • FocalLoss - applies masking after one-hot conversion
  • TverskyLoss - consistent masking approach with DiceLoss
  • UnifiedFocalLoss - handles ignore_index in binary target conversion

Metrics with ignore_index support:

  • MeanDice & GeneralizedDiceScore - spatial masking before computation
  • MeanIoU - excludes ignored classes from IoU calculation
  • ConfusionMatrixMetric - filters ignored indices from confusion matrix
  • HausdorffDistanceMetric - masks boundary computation
  • SurfaceDiceMetric & SurfaceDistanceMetric - handles ignore_index in surface calculations

Key Fixes:

  • Added support for ignore_index
  • Added comprehensive test coverage in test_ignore_index_losses.py and test_ignore_index_metrics.py

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality)
  • Breaking change (fix or new feature that would cause existing functionality to change)
  • New tests added to cover the changes (test_ignore_index_losses.py, test_ignore_index_metrics.py)
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests
  • In-line docstrings updated
  • Documentation updated, tested make html command in the docs/ folder

Related Issues

Could merge with or addresses similar requirements as #8667 (Ignore Class)

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

This pull request adds native ignore_index parameter support across MONAI's loss functions and metrics. The parameter allows users to exclude specific class indices (e.g., padding, unlabeled regions) from computations. Changes span DiceLoss, FocalLoss, TverskyLoss, related focal loss classes, and metrics including DiceMetric, MeanIoU, GeneralizedDiceScore, ConfusionMatrixMetric, HausdorffDistanceMetric, SurfaceDistanceMetric, and SurfaceDiceMetric. Implementation follows a consistent pattern: add ignore_index parameter to constructors, store as instance attribute, compute a mask excluding the specified index, and apply the mask to inputs before loss/metric computation. Utility functions in monai/metrics/utils.py are extended to support masking operations. Two new test modules validate ignore_index behavior across losses and metrics.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 74.51% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive Title is truncated and incomplete ('implement ignore_index support across dice and …'), making it unclear what the full scope of changes is. Provide complete PR title that fully describes the feature implementation without truncation.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed PR description is comprehensive, covers all changes across losses and metrics, includes linked issue reference, test information, and fulfills template requirements.
Linked Issues check ✅ Passed All core objectives from issue #8734 are met: ignore_index parameter added to losses and metrics, voxels with ignored labels are masked before computations, and implementation works with reduction modes.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing ignore_index support across losses and metrics as specified in issue #8734; no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Rusheel86 Rusheel86 force-pushed the feat-ignore-index-support branch 6 times, most recently from 7177b8d to b74ef93 Compare March 1, 2026 18:08
…ated unit tests

Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
@Rusheel86 Rusheel86 force-pushed the feat-ignore-index-support branch from b74ef93 to f2caaf8 Compare March 9, 2026 14:43
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
I, Rusheel Sharma <rusheelhere@gmail.com>, hereby add my Signed-off-by to this commit: d075009

Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
Signed-off-by: Rusheel Sharma <rusheelhere@gmail.com>
@Rusheel86 Rusheel86 marked this pull request as ready for review March 10, 2026 17:29
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
monai/losses/focal_loss.py (1)

150-154: ⚠️ Potential issue | 🔴 Critical

ignore_index is derived from the wrong target representation.

The comparison happens after any one_hot conversion, so it only works for sentinel-filled tensors. For documented one-hot targets, class IDs are gone and ignore_index never identifies the ignored voxels. Keep the pre-conversion labels for masking, and fall back to the ignored class channel when the target is already one-hot.

Also applies to: 167-170

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/losses/focal_loss.py` around lines 150 - 154, The ignore_index mask is
being computed from the post-conversion target (after one_hot), so when users
pass one-hot targets the class IDs are lost and ignored voxels aren’t detected;
fix by saving the original target (e.g., orig_target = target) before calling
one_hot in the method where self.to_onehot_y is handled, compute the ignore mask
from orig_target when it contains integer class labels, and if the supplied
target is already one-hot derive the mask from the ignore_index channel (e.g.,
orig_target[..., ignore_index] or equivalent); apply the same change to the
second occurrence of the logic (lines matching the 167-170 block) so both paths
correctly detect ignored voxels regardless of whether target was converted or
already one-hot.
monai/losses/dice.py (1)

169-177: ⚠️ Potential issue | 🔴 Critical

Build the ignore mask from a sanitized label view.

At Line 171 the mask is based on tensor values, so it only works for single-channel sentinel maps. For documented one-hot targets, ignore_index never matches class membership, and with include_background=False the C-channel mask no longer broadcasts after Lines 183-185 slice input and target. Also sanitize ignored labels before Line 177’s one_hot.

Also applies to: 179-192

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/losses/dice.py` around lines 169 - 177, Build the ignore mask from a
sanitized label view before converting to one-hot: compute a boolean label_mask
= (target != self.ignore_index) (or all-True if ignore_index is None), then if
self.to_onehot_y is True sanitize target for one_hot by copying target and
setting ignored positions to a valid class (e.g., 0) before calling one_hot;
after one_hot, expand/reshape label_mask into the channel dimension to match
target/prediction (e.g., mask =
label_mask.unsqueeze(1).expand_as(target).float()) so the mask works for both
single-channel sentinel maps and multi-channel one-hot targets and still
broadcasts correctly when include_background slicing happens (apply same mask
generation in the later block covering lines ~179-192).
monai/metrics/utils.py (3)

316-326: ⚠️ Potential issue | 🟡 Minor

ignore_index parameter unused in get_edge_surface_distance.

Parameter at line 325 is accepted but not forwarded to get_mask_edges at line 355 or used elsewhere in the function.

🛠️ Forward ignore_index to get_mask_edges
-    edge_results = get_mask_edges(y_pred, y, crop=True, spacing=edges_spacing, always_return_as_numpy=False)
+    edge_results = get_mask_edges(y_pred, y, crop=True, spacing=edges_spacing, always_return_as_numpy=False, ignore_index=ignore_index)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/metrics/utils.py` around lines 316 - 326, The get_edge_surface_distance
function accepts ignore_index but never forwards it to get_mask_edges or uses
it; update get_edge_surface_distance to pass the ignore_index argument through
when calling get_mask_edges (the call site around get_mask_edges in
get_edge_surface_distance) so masked edge computation respects ignore_index, and
ensure any signatures (get_mask_edges invocation) and downstream uses handle a
None value appropriately.

265-271: ⚠️ Potential issue | 🟡 Minor

mask parameter added but unused.

The mask parameter at line 270 is not used in get_surface_distance. The function computes distances based on edges but doesn't apply the mask. Remove or implement.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/metrics/utils.py` around lines 265 - 271, get_surface_distance
currently accepts a mask parameter but never uses it; either remove the
parameter or apply it to limit where surface distances are computed. To fix,
update get_surface_distance so that before computing edges (from seg_pred and
seg_gt) you validate and cast mask to a boolean array matching seg_pred/seg_gt
shape and then apply it to zero-out/ignore voxels outside the mask (e.g., mask
the inputs seg_pred and seg_gt or filter edge point sets) so that subsequent
edge extraction and distance calculations only consider masked regions; keep the
existing logic for distance_metric and spacing unchanged and ensure mask
handling is documented in the function signature and tests.

161-169: ⚠️ Potential issue | 🟡 Minor

ignore_index parameter is unused and undocumented.

The parameter at line 168 is never referenced in the function body and missing from the docstring's Args section. Either implement the functionality and document it, or remove the parameter to keep the API honest.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/metrics/utils.py` around lines 161 - 169, The ignore_index parameter in
get_mask_edges is never used and not documented; remove it from the function
signature and any related type hints, delete its mention in the docstring Args
section, and update any call sites/tests that pass ignore_index to stop
supplying it; alternatively if you prefer keeping behavior, implement logic
inside get_mask_edges to mask out label==ignore_index from seg_gt/seg_pred
before processing and document the parameter in the docstring—choose one
approach and make matching changes to callers and tests for consistency.
🧹 Nitpick comments (2)
monai/metrics/hausdorff_distance.py (1)

199-207: Redundant masking when both ignore_index and mask are used.

At line 105-107, y_pred and y are already masked. Then at lines 200-202, yp and yt are masked again with valid_mask. The mask[b, 0] is also passed to get_edge_surface_distance at line 215. This triple application may be intentional for robustness but adds overhead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/metrics/hausdorff_distance.py` around lines 199 - 207, The code
redundantly reapplies masking: y_pred/y are already masked earlier (mask[b, 0])
yet yp/yt are masked again with valid_mask before computing distances and the
same mask is later passed into get_edge_surface_distance; remove the duplicated
masking to avoid overhead by eliminating the yp = yp * valid_mask and yt = yt *
valid_mask lines (or skip the earlier mask and only apply a single, consistent
mask) and ensure the mask passed to get_edge_surface_distance is the same single
mask used to prepare yp/yt; update references in the loop that set hd[b, c] and
the early all-ignored check to use that single mask (valid_mask or mask[b, 0])
so masking occurs exactly once.
monai/metrics/generalized_dice.py (1)

198-204: Per-batch loop for inf replacement is functional but could be vectorized.

Works correctly but iterating per batch may be slower for large batches.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/metrics/generalized_dice.py` around lines 198 - 204, Loop over batches
replacing infs in w_full is correct but can be vectorized: compute inf_mask =
torch.isinf(w_full), create temp = w_full.masked_fill(inf_mask, 0), compute
per-batch max_w = temp.view(w_full.shape[0], -1).max(dim=1).values, build
per-batch replacement values = torch.where(max_w > 0, max_w, torch.tensor(1.0,
device=w_full.device)), expand/broadcast those replacements to the shape of
w_full and assign them into w_full at inf_mask; update the code replacing the
per-batch for-loop (variables: w_full, batch_w, infs) with this vectorized
sequence so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@monai/losses/focal_loss.py`:
- Around line 167-171: The current implementation applies the ignore mask to
logits/targets (input/target) which still yields a nonzero BCE/focal term for
masked entries; instead, compute the elementwise focal/BCE loss normally (in the
sigmoid and other paths) and then multiply that per-element loss by mask to zero
out ignored voxels, and when reducing use sum(loss * mask) / mask.sum() (or
clamp minimal denom) so ignored elements don't contribute to numerator or
denominator; update the forward method in FocalLoss (and the other masked blocks
referenced around the sigmoid branch and the regions you noted at 183-185 and
207-218) to apply mask to the computed loss tensor and perform masked reduction
rather than masking input/target or using .mean() over masked values.

In `@monai/losses/tversky.py`:
- Around line 135-136: original_target is preserved but one_hot is called on the
raw target which can contain out-of-range sentinel values (e.g., 255) causing
one_hot to fail; before calling one_hot(target, num_classes=n_pred_ch) replace
ignored/sentinel labels in target with a valid class id (e.g., 0 or another safe
class index) then call one_hot using that sanitized target, and continue to
derive the ignore mask from original_target (not the sanitized target) when
constructing the mask and applying it in the subsequent loss logic (refer to
original_target, target, one_hot, n_pred_ch, and the mask construction used in
the block around lines 138-148).

In `@monai/losses/unified_focal_loss.py`:
- Around line 167-172: The mean-reduction branch incorrectly divides loss.sum()
by spatial_mask.sum(), undercounting channels (loss has shape [B,2,H,W]); update
the normalization to account for both channel elements by either applying the
spatial mask to loss (e.g., expand spatial_mask to match loss with
spatial_mask.unsqueeze(1).expand_as(loss)), summing the masked loss and dividing
by the masked-element count, or divide loss.sum() by (spatial_mask.sum() *
loss.size(1)). Make the change in the block handling self.reduction ==
LossReduction.MEAN.value and self.ignore_index is not None (affecting variables
loss, spatial_mask, back_ce, fore_ce).
- Around line 71-77: The current mask uses (torch.sum(y_true, dim=1) > 0) which
does not respect ignore_index; update the ignore handling to build the mask from
the original label indices or from the explicit ignored-class channel instead of
summing one-hot channels. Concretely: when ignore_index is set, if you have
access to the original label tensor (e.g., labels / y_true_indices) create
valid_mask = (labels != self.ignore_index).unsqueeze(1) and expand it to y_true
shape; otherwise (when y_true is one-hot) build the mask from the ignored class
channel as valid_mask = 1 - y_true[:, self.ignore_index:self.ignore_index+1,
...] and use that to mask y_pred and other computations (replace spatial_mask
usage in this file and in AsymmetricFocalTverskyLoss code paths).
- Around line 157-160: The ignore_index handling currently builds spatial_mask
from torch.sum(y_true, dim=1) which fails when self.to_onehot_y is False; update
the block in AsymmetricFocalLoss (unified_focal_loss.py) to branch on
self.to_onehot_y: when to_onehot_y is False compute spatial_mask = (y_true !=
self.ignore_index).unsqueeze(1).float() (so label tensors drop ignored voxels),
otherwise keep the existing one-hot style mask (e.g., spatial_mask =
(torch.sum(y_true, dim=1, keepdim=True) > 0).float()); then apply that
spatial_mask to cross_entropy as before.

In `@monai/metrics/generalized_dice.py`:
- Around line 157-161: The current ignore_index mask in
monai/metrics/generalized_dice.py compares a one-hot y to an integer and always
yields True; change the masking to detect one-hot class vectors using n_class:
if ignore_index >= n_class set mask = (y.sum(dim=1, keepdim=True) > 0).float(),
else set mask = (1.0 - y[:, ignore_index:ignore_index+1]).float(); then apply
that mask to y_pred and y as before (y_pred = y_pred * mask; y = y * mask). Use
the existing variables y, y_pred, ignore_index and n_class to implement this
branching so one-hot inputs are masked correctly.

In `@monai/metrics/hausdorff_distance.py`:
- Around line 151-152: The docstring for hausdorff_distance is missing
documentation for the mask parameter; update the function's docstring to
describe mask: its type (torch.Tensor | None), expected shape (same spatial
shape as y_pred/y), semantics (binary or boolean tensor where True/non-zero
marks voxels to include in the metric; if None, no spatial masking is applied),
how it interacts with ignore_index (e.g., masked-out voxels are excluded before
applying ignore_index), and any dtype/format expectations (boolean or 0/1
integer). Mention that mask applies to both prediction and ground-truth when
computing distances and that mask and ignore_index can be used together (specify
precedence if relevant).

In `@monai/metrics/meaniou.py`:
- Around line 146-151: compute_iou's ignore_index handling assumes label-encoded
y but must support one-hot y; change the mask logic in compute_iou to detect
one-hot (y.dim()==y_pred.dim() and y.shape[1]==y_pred.shape[1] or otherwise by
checking channel count) and, when one-hot, build a [B,1,...] mask from y[:,
ignore_index:ignore_index+1] (e.g., mask = 1 - y[:,
ignore_index:ignore_index+1]) then expand_as(y_pred) and apply it to y_pred; for
label-encoded y keep the existing mask = (y != ignore_index).float() and replace
y values with 0 via torch.where(y == ignore_index, torch.tensor(0,
device=y.device), y) only in that branch so one-hot y is not altered
incorrectly.

In `@monai/metrics/surface_dice.py`:
- Around line 223-227: The current one-hot mask logic using (y !=
ignore_index).all(dim=1, keepdim=True) is incorrect for one-hot encoded labels;
replace it by constructing a class-based mask from the one-hot channel: build
mask = (1 - y[:, ignore_index:ignore_index+1]).float() (preserving a [B,1,...]
shape) and then multiply y_pred and y by that mask; update the block that checks
ignore_index and applies masking to y_pred and y (referencing y, y_pred, and
ignore_index) so ignored-class voxels are zeroed correctly.

In `@monai/metrics/surface_distance.py`:
- Around line 174-176: The current conditional skips calling ignore_background
when include_background is False and ignore_index == 0, causing background
removal to be bypassed unintentionally; update the logic in surface_distance.py
so that when include_background is False you always call
ignore_background(y_pred=y_pred, y=y) (remove or change the ignore_index check),
or alternatively make the behavior explicit by documenting that ignore_index==0
preserves background; reference the include_background flag, ignore_index
variable, and the ignore_background(y_pred=y_pred, y=y) call when making the
change.

In `@monai/metrics/utils.py`:
- Around line 358-364: The mask-slicing branch currently only runs when
edge_results[2] is a tuple, so in subvoxel mode (when spacing is provided and
edge_results[2] is an area tensor) the mask is never applied; update the code
around edge_results/ mask handling so it handles both cases: if edge_results[2]
is a tuple extract slices and do mask = mask[slices], otherwise skip slicing and
just move/cast the mask to the edges_pred device (mask =
mask.to(edges_pred.device).bool()), then apply edges_pred = edges_pred & mask
and edges_gt = edges_gt & mask. Ensure you reference edge_results, mask,
edges_pred and edges_gt when making the change.

In `@tests/losses/test_ignore_index_losses.py`:
- Around line 22-27: The current TEST_CASES only checks invariance with ignored
indices but not that ignored voxels are truly excluded; update the tests in
tests/losses/test_ignore_index_losses.py to compute each loss twice—(1) the
existing full-volume loss with ignore_index set and (2) a reference loss
computed only on the non-ignored region (mask out ignored voxels from y_true and
y_pred and/or slice to the non-ignored voxels) and assert those values match—so
constant penalties from ignored voxels would fail; extend TEST_CASES to include
both to_onehot_y=True and an already-one-hot label case for each Loss class
(DiceLoss, FocalLoss, TverskyLoss, AsymmetricUnifiedFocalLoss) and ensure the
test covers both softmax/sigmoid variants referenced in the tuple entries.

In `@tests/metrics/test_ignore_index_metrics.py`:
- Around line 48-50: The class-level `@unittest.skipUnless`(has_scipy, ...) is
incorrectly skipping metrics that don't require SciPy; update the test split so
non-SciPy metrics run regardless. Remove or change the decorator on
TestIgnoreIndexMetrics and have it use `@parameterized.expand`(TEST_METRICS) only,
then create a separate test class (e.g., TestIgnoreIndexSurfaceMetrics)
decorated with `@unittest.skipUnless`(has_scipy, ...) that uses
`@parameterized.expand`(SCIPY_METRICS) so only the surface-distance tests are
gated on has_scipy.
- Around line 63-66: The test currently fills all channels with 255 which never
triggers the class-based one-hot path; change the bottom-half sentinel
assignment to a proper one-hot vector for the ignored class so the masking logic
is exercised—specifically, update the tensor y (created in the test) so that for
the ignored class channel (referenced as the channel index used by the
metric/ignored_class constant) you set y[:, ignored_channel, 2:4, :] = 1 and
ensure the other channels in that region remain 0; keep the top-half valid
labels (y[:, 1, 0:2, 0:2] = 1.0) intact so both valid and ignored-class regions
are present and the class-based one-hot branch in the metric (the one handling
one-hot label tensors) is covered.

---

Outside diff comments:
In `@monai/losses/dice.py`:
- Around line 169-177: Build the ignore mask from a sanitized label view before
converting to one-hot: compute a boolean label_mask = (target !=
self.ignore_index) (or all-True if ignore_index is None), then if
self.to_onehot_y is True sanitize target for one_hot by copying target and
setting ignored positions to a valid class (e.g., 0) before calling one_hot;
after one_hot, expand/reshape label_mask into the channel dimension to match
target/prediction (e.g., mask =
label_mask.unsqueeze(1).expand_as(target).float()) so the mask works for both
single-channel sentinel maps and multi-channel one-hot targets and still
broadcasts correctly when include_background slicing happens (apply same mask
generation in the later block covering lines ~179-192).

In `@monai/losses/focal_loss.py`:
- Around line 150-154: The ignore_index mask is being computed from the
post-conversion target (after one_hot), so when users pass one-hot targets the
class IDs are lost and ignored voxels aren’t detected; fix by saving the
original target (e.g., orig_target = target) before calling one_hot in the
method where self.to_onehot_y is handled, compute the ignore mask from
orig_target when it contains integer class labels, and if the supplied target is
already one-hot derive the mask from the ignore_index channel (e.g.,
orig_target[..., ignore_index] or equivalent); apply the same change to the
second occurrence of the logic (lines matching the 167-170 block) so both paths
correctly detect ignored voxels regardless of whether target was converted or
already one-hot.

In `@monai/metrics/utils.py`:
- Around line 316-326: The get_edge_surface_distance function accepts
ignore_index but never forwards it to get_mask_edges or uses it; update
get_edge_surface_distance to pass the ignore_index argument through when calling
get_mask_edges (the call site around get_mask_edges in
get_edge_surface_distance) so masked edge computation respects ignore_index, and
ensure any signatures (get_mask_edges invocation) and downstream uses handle a
None value appropriately.
- Around line 265-271: get_surface_distance currently accepts a mask parameter
but never uses it; either remove the parameter or apply it to limit where
surface distances are computed. To fix, update get_surface_distance so that
before computing edges (from seg_pred and seg_gt) you validate and cast mask to
a boolean array matching seg_pred/seg_gt shape and then apply it to
zero-out/ignore voxels outside the mask (e.g., mask the inputs seg_pred and
seg_gt or filter edge point sets) so that subsequent edge extraction and
distance calculations only consider masked regions; keep the existing logic for
distance_metric and spacing unchanged and ensure mask handling is documented in
the function signature and tests.
- Around line 161-169: The ignore_index parameter in get_mask_edges is never
used and not documented; remove it from the function signature and any related
type hints, delete its mention in the docstring Args section, and update any
call sites/tests that pass ignore_index to stop supplying it; alternatively if
you prefer keeping behavior, implement logic inside get_mask_edges to mask out
label==ignore_index from seg_gt/seg_pred before processing and document the
parameter in the docstring—choose one approach and make matching changes to
callers and tests for consistency.

---

Nitpick comments:
In `@monai/metrics/generalized_dice.py`:
- Around line 198-204: Loop over batches replacing infs in w_full is correct but
can be vectorized: compute inf_mask = torch.isinf(w_full), create temp =
w_full.masked_fill(inf_mask, 0), compute per-batch max_w =
temp.view(w_full.shape[0], -1).max(dim=1).values, build per-batch replacement
values = torch.where(max_w > 0, max_w, torch.tensor(1.0, device=w_full.device)),
expand/broadcast those replacements to the shape of w_full and assign them into
w_full at inf_mask; update the code replacing the per-batch for-loop (variables:
w_full, batch_w, infs) with this vectorized sequence so behavior remains
identical.

In `@monai/metrics/hausdorff_distance.py`:
- Around line 199-207: The code redundantly reapplies masking: y_pred/y are
already masked earlier (mask[b, 0]) yet yp/yt are masked again with valid_mask
before computing distances and the same mask is later passed into
get_edge_surface_distance; remove the duplicated masking to avoid overhead by
eliminating the yp = yp * valid_mask and yt = yt * valid_mask lines (or skip the
earlier mask and only apply a single, consistent mask) and ensure the mask
passed to get_edge_surface_distance is the same single mask used to prepare
yp/yt; update references in the loop that set hd[b, c] and the early all-ignored
check to use that single mask (valid_mask or mask[b, 0]) so masking occurs
exactly once.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: de4d36bb-f303-47c9-bc69-6339e0ccdc88

📥 Commits

Reviewing files that changed from the base of the PR and between daaedaa and af83422.

📒 Files selected for processing (14)
  • monai/losses/dice.py
  • monai/losses/focal_loss.py
  • monai/losses/tversky.py
  • monai/losses/unified_focal_loss.py
  • monai/metrics/confusion_matrix.py
  • monai/metrics/generalized_dice.py
  • monai/metrics/hausdorff_distance.py
  • monai/metrics/meandice.py
  • monai/metrics/meaniou.py
  • monai/metrics/surface_dice.py
  • monai/metrics/surface_distance.py
  • monai/metrics/utils.py
  • tests/losses/test_ignore_index_losses.py
  • tests/metrics/test_ignore_index_metrics.py

Comment on lines +167 to +171
if self.ignore_index is not None:
mask = (target != self.ignore_index).float()
input = input * mask
target = target * mask

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Mask the focal loss, not the logits.

In the sigmoid path, input = 0 and target = 0 still produce a positive BCE/focal term, so ignored voxels are turned into background loss instead of being excluded. loss.mean() also continues to divide by ignored elements. Apply the mask to the elementwise loss, then reduce over valid elements only.

Also applies to: 183-185, 207-218

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/losses/focal_loss.py` around lines 167 - 171, The current
implementation applies the ignore mask to logits/targets (input/target) which
still yields a nonzero BCE/focal term for masked entries; instead, compute the
elementwise focal/BCE loss normally (in the sigmoid and other paths) and then
multiply that per-element loss by mask to zero out ignored voxels, and when
reducing use sum(loss * mask) / mask.sum() (or clamp minimal denom) so ignored
elements don't contribute to numerator or denominator; update the forward method
in FocalLoss (and the other masked blocks referenced around the sigmoid branch
and the regions you noted at 183-185 and 207-218) to apply mask to the computed
loss tensor and perform masked reduction rather than masking input/target or
using .mean() over masked values.

Comment on lines +135 to 136
original_target = target
target = one_hot(target, num_classes=n_pred_ch)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Sanitize ignored labels before one_hot.

original_target is saved at Line 135, but Line 136 still one-hot encodes the raw labels. If callers use an out-of-range sentinel such as 255, conversion fails before the mask at Lines 138-148 runs. Replace ignored voxels with a valid class ID before one_hot, then derive the mask from original_target.

Also applies to: 138-148

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/losses/tversky.py` around lines 135 - 136, original_target is preserved
but one_hot is called on the raw target which can contain out-of-range sentinel
values (e.g., 255) causing one_hot to fail; before calling one_hot(target,
num_classes=n_pred_ch) replace ignored/sentinel labels in target with a valid
class id (e.g., 0 or another safe class index) then call one_hot using that
sanitized target, and continue to derive the ignore mask from original_target
(not the sanitized target) when constructing the mask and applying it in the
subsequent loss logic (refer to original_target, target, one_hot, n_pred_ch, and
the mask construction used in the block around lines 138-148).

Comment on lines +71 to +77
# Handle ignore_index:
mask = torch.ones_like(y_true)
if self.ignore_index is not None:
# Identify valid pixels: where at least one channel is 1
spatial_mask = (torch.sum(y_true, dim=1, keepdim=True) > 0).float()
mask = spatial_mask.expand_as(y_true)
y_pred = y_pred * mask
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

ignore_index is not used to build this mask.

sum(y_true, dim=1) > 0 marks every normal one-hot voxel as valid, regardless of which class should be ignored, and it also keeps sentinel-filled ignored voxels (255 + 255 > 0). The standalone AsymmetricFocalTverskyLoss path therefore still includes ignored regions. Build the mask from the original labels, or from the ignored class channel.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/losses/unified_focal_loss.py` around lines 71 - 77, The current mask
uses (torch.sum(y_true, dim=1) > 0) which does not respect ignore_index; update
the ignore handling to build the mask from the original label indices or from
the explicit ignored-class channel instead of summing one-hot channels.
Concretely: when ignore_index is set, if you have access to the original label
tensor (e.g., labels / y_true_indices) create valid_mask = (labels !=
self.ignore_index).unsqueeze(1) and expand it to y_true shape; otherwise (when
y_true is one-hot) build the mask from the ignored class channel as valid_mask =
1 - y_true[:, self.ignore_index:self.ignore_index+1, ...] and use that to mask
y_pred and other computations (replace spatial_mask usage in this file and in
AsymmetricFocalTverskyLoss code paths).

Comment on lines +157 to +160
if self.ignore_index is not None:
spatial_mask = (torch.sum(y_true, dim=1, keepdim=True) > 0).float()
cross_entropy = cross_entropy * spatial_mask

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

The default AsymmetricFocalLoss path still includes ignored voxels.

With to_onehot_y=False, this again uses sum(y_true) > 0, so one-hot targets never drop a specific class and sentinel-filled ignored regions stay valid. Since that is the default mode, ignore_index is ineffective for the standalone loss.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/losses/unified_focal_loss.py` around lines 157 - 160, The ignore_index
handling currently builds spatial_mask from torch.sum(y_true, dim=1) which fails
when self.to_onehot_y is False; update the block in AsymmetricFocalLoss
(unified_focal_loss.py) to branch on self.to_onehot_y: when to_onehot_y is False
compute spatial_mask = (y_true != self.ignore_index).unsqueeze(1).float() (so
label tensors drop ignored voxels), otherwise keep the existing one-hot style
mask (e.g., spatial_mask = (torch.sum(y_true, dim=1, keepdim=True) >
0).float()); then apply that spatial_mask to cross_entropy as before.

Comment on lines +167 to +172
loss = torch.stack([back_ce, fore_ce], dim=1) # [B, 2, H, W]
if self.reduction == LossReduction.MEAN.value:
if self.ignore_index is not None:
# Normalize by the number of non-ignored pixels
return loss.sum() / spatial_mask.sum().clamp(min=1e-5)
return loss.mean()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

mean reduction is scaled by pixels, not valid elements.

loss.sum() / spatial_mask.sum() is 2x larger than loss.mean() when nothing is ignored because loss has two channels but the denominator counts only pixels. Divide by the expanded mask sum, or average loss after masking it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/losses/unified_focal_loss.py` around lines 167 - 172, The
mean-reduction branch incorrectly divides loss.sum() by spatial_mask.sum(),
undercounting channels (loss has shape [B,2,H,W]); update the normalization to
account for both channel elements by either applying the spatial mask to loss
(e.g., expand spatial_mask to match loss with
spatial_mask.unsqueeze(1).expand_as(loss)), summing the masked loss and dividing
by the masked-element count, or divide loss.sum() by (spatial_mask.sum() *
loss.size(1)). Make the change in the block handling self.reduction ==
LossReduction.MEAN.value and self.ignore_index is not None (affecting variables
loss, spatial_mask, back_ce, fore_ce).

Comment on lines 174 to +176
if not include_background:
y_pred, y = ignore_background(y_pred=y_pred, y=y)
if ignore_index != 0:
y_pred, y = ignore_background(y_pred=y_pred, y=y)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Edge case: ignore_index=0 skips background removal entirely.

When ignore_index=0, line 175 evaluates to False, so ignore_background is never called even if include_background=False. This means setting ignore_index=0 overrides include_background=False, which may be intentional but should be documented.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/metrics/surface_distance.py` around lines 174 - 176, The current
conditional skips calling ignore_background when include_background is False and
ignore_index == 0, causing background removal to be bypassed unintentionally;
update the logic in surface_distance.py so that when include_background is False
you always call ignore_background(y_pred=y_pred, y=y) (remove or change the
ignore_index check), or alternatively make the behavior explicit by documenting
that ignore_index==0 preserves background; reference the include_background
flag, ignore_index variable, and the ignore_background(y_pred=y_pred, y=y) call
when making the change.

Comment on lines +358 to +364
if mask is not None:
if len(edge_results) > 2 and isinstance(edge_results[2], tuple):
slices = edge_results[2]
mask = mask[slices]
mask = mask.to(edges_pred.device).bool()
edges_pred = edges_pred & mask
edges_gt = edges_gt & mask
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check get_mask_edges return structure
rg -n "return.*edges" monai/metrics/utils.py -B 2 -A 2

Repository: Project-MONAI/MONAI

Length of output: 859


🏁 Script executed:

rg -n "def get_mask_edges" monai/metrics/utils.py -A 50

Repository: Project-MONAI/MONAI

Length of output: 2700


🏁 Script executed:

sed -n '340,365p' monai/metrics/utils.py

Repository: Project-MONAI/MONAI

Length of output: 1280


🏁 Script executed:

rg -n "return" monai/metrics/utils.py | grep -A 2 -B 2 "161\|162\|163\|164\|165"

Repository: Project-MONAI/MONAI

Length of output: 45


🏁 Script executed:

sed -n '161,330p' monai/metrics/utils.py

Repository: Project-MONAI/MONAI

Length of output: 8350


🏁 Script executed:

# Check if CropForegroundD or any other part returns slices
rg -n "slices" monai/metrics/utils.py

Repository: Project-MONAI/MONAI

Length of output: 139


🏁 Script executed:

rg -n "class CropForegroundD" monai/

Repository: Project-MONAI/MONAI

Length of output: 45


Mask slicing condition will never execute in subvoxel mode.

Line 359 checks isinstance(edge_results[2], tuple), but when spacing is provided, edge_results[2] contains an area tensor, not slices. The mask application at lines 360-361 will be skipped, leaving masks unapplied in subvoxel mode.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@monai/metrics/utils.py` around lines 358 - 364, The mask-slicing branch
currently only runs when edge_results[2] is a tuple, so in subvoxel mode (when
spacing is provided and edge_results[2] is an area tensor) the mask is never
applied; update the code around edge_results/ mask handling so it handles both
cases: if edge_results[2] is a tuple extract slices and do mask = mask[slices],
otherwise skip slicing and just move/cast the mask to the edges_pred device
(mask = mask.to(edges_pred.device).bool()), then apply edges_pred = edges_pred &
mask and edges_gt = edges_gt & mask. Ensure you reference edge_results, mask,
edges_pred and edges_gt when making the change.

Comment on lines +22 to +27
# Defining test cases: (LossClass, args)
TEST_CASES = [
(DiceLoss, {"sigmoid": True}),
(FocalLoss, {"use_softmax": False}),
(TverskyLoss, {"sigmoid": True}),
(AsymmetricUnifiedFocalLoss, {}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

This only checks invariance, not actual exclusion.

A loss can still add a constant penalty from ignored voxels and pass this test as long as both inputs collapse to the same value there. Compare against a reference loss computed on just the non-ignored region, and add to_onehot_y=True / already-one-hot cases so the new paths are covered. As per coding guidelines, "Ensure new or modified definitions will be covered by existing or new unit tests."

Also applies to: 33-54

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/losses/test_ignore_index_losses.py` around lines 22 - 27, The current
TEST_CASES only checks invariance with ignored indices but not that ignored
voxels are truly excluded; update the tests in
tests/losses/test_ignore_index_losses.py to compute each loss twice—(1) the
existing full-volume loss with ignore_index set and (2) a reference loss
computed only on the non-ignored region (mask out ignored voxels from y_true and
y_pred and/or slice to the non-ignored voxels) and assert those values match—so
constant penalties from ignored voxels would fail; extend TEST_CASES to include
both to_onehot_y=True and an already-one-hot label case for each Loss class
(DiceLoss, FocalLoss, TverskyLoss, AsymmetricUnifiedFocalLoss) and ensure the
test covers both softmax/sigmoid variants referenced in the tuple entries.

Comment on lines +48 to +50
@unittest.skipUnless(has_scipy, "Scipy required for surface metrics")
class TestIgnoreIndexMetrics(unittest.TestCase):
@parameterized.expand(TEST_METRICS + SCIPY_METRICS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't gate the non-SciPy metrics behind SciPy.

This decorator skips DiceMetric, MeanIoU, GeneralizedDiceScore, and ConfusionMatrixMetric whenever SciPy is absent, even though they do not depend on it. Split the surface-distance cases into a separate skipped class, or skip only those parameter sets. As per coding guidelines, "Ensure new or modified definitions will be covered by existing or new unit tests."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/metrics/test_ignore_index_metrics.py` around lines 48 - 50, The
class-level `@unittest.skipUnless`(has_scipy, ...) is incorrectly skipping metrics
that don't require SciPy; update the test split so non-SciPy metrics run
regardless. Remove or change the decorator on TestIgnoreIndexMetrics and have it
use `@parameterized.expand`(TEST_METRICS) only, then create a separate test class
(e.g., TestIgnoreIndexSurfaceMetrics) decorated with
`@unittest.skipUnless`(has_scipy, ...) that uses
`@parameterized.expand`(SCIPY_METRICS) so only the surface-distance tests are
gated on has_scipy.

Comment on lines +63 to +66
# Target: Top half is valid (0/1), Bottom half is 255
y = torch.zeros((1, 2, 4, 4))
y[:, 1, 0:2, 0:2] = 1.0
y[:, :, 2:4, :] = 255
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

These fixtures never hit the class-based one-hot path.

Writing 255 into every channel only tests a sentinel-filled tensor. The metric APIs here expect one-hot labels, so add a case where the ignored class channel is 1 and the others are 0; otherwise the new masking logic is not exercised. As per coding guidelines, "Ensure new or modified definitions will be covered by existing or new unit tests."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/metrics/test_ignore_index_metrics.py` around lines 63 - 66, The test
currently fills all channels with 255 which never triggers the class-based
one-hot path; change the bottom-half sentinel assignment to a proper one-hot
vector for the ignored class so the masking logic is exercised—specifically,
update the tensor y (created in the test) so that for the ignored class channel
(referenced as the channel index used by the metric/ignored_class constant) you
set y[:, ignored_channel, 2:4, :] = 1 and ensure the other channels in that
region remain 0; keep the top-half valid labels (y[:, 1, 0:2, 0:2] = 1.0) intact
so both valid and ignored-class regions are present and the class-based one-hot
branch in the metric (the one handling one-hot label tensors) is covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Support ignore_index / ignore_class in Losses and Metrics

1 participant